Hi Wouter and Dmitri,

Based on the comments received this morning at the Reconstruction WG meeting, I updated the branch with the general calculation for any two-track combinations. I tested with the pi-pi selection in the down-stream analysis and it yields identical results compared to my early implementation.

Capybara summary for PR 2144

• rec_dis_10x100_minQ2=0_craterlake_10x100
• rec_dis_10x100_minQ2=1000_craterlake_tracking_only
• rec_dis_18x275_minQ2=0_craterlake_18x275
• rec_dis_18x275_minQ2=0_craterlake_18x275_MT
• rec_dis_18x275_minQ2=0_craterlake_18x275_ST_MT
• rec_dis_18x275_minQ2=1000_craterlake_18x275
• rec_dis_18x275_minQ2=1000_craterlake_18x275_MT
• rec_dis_18x275_minQ2=1000_craterlake_18x275_ST_MT
• rec_dis_5x41_minQ2=0_craterlake_5x41
• rec_e_1GeV_20GeV_craterlake
• rec_pi_1GeV_20GeV_craterlake

Last updated 2025-11-25T10:17-05:00 60948e8

Hi Wouter and Dmitri,

The two remaining failed checks seem to beyond my knowledge. Will you be able to help identify what went wrong there? Thanks and Regards

/xin

Hi Wouter and Dmitri,

The two remaining failed checks seem to beyond my knowledge. Will you be able to help identify what went wrong there? Thanks and Regards

/xin

This looks like an irrelevant issue. Should resolve itself once latest geometry makes it into the container. I'm rerunning container build without cache now, so, hopefully, this will clear away. We can review the code meanwhile.

Hi Wouter and Dmitri,
The two remaining failed checks seem to beyond my knowledge. Will you be able to help identify what went wrong there? Thanks and Regards
/xin

This looks like an irrelevant issue. Should resolve itself once latest geometry makes it into the container. I'm rerunning container build without cache now, so, hopefully, this will clear away. We can review the code meanwhile.

Hi Dmitri and Wouter,

Do you have any suggestions/comments to this PR? Any adjustment is needed before it gets merged? Thanks

/xin

This is on my radar, but I haven't had chance to look deeper yet. I can share a few early comments. We don't have a convention for utility libraries in EICrecon. For now, do you think we could merge Helix.cc and Helix.h into SecondaryVerticesHelix.cc? I also see you provide Config structure, but don't expose the parameters in the factory. And last is the hardcoded b_field, would it make sense to get the value from the geometry service instead (e.g. sample at the 0,0,0 coordinate)?

This is on my radar, but I haven't had chance to look deeper yet. I can share a few early comments. We don't have a convention for utility libraries in EICrecon. For now, do you think we could merge Helix.cc and Helix.h into SecondaryVerticesHelix.cc? I also see you provide Config structure, but don't expose the parameters in the factory. And last is the hardcoded b_field, would it make sense to get the value from the geometry service instead (e.g. sample at the 0,0,0 coordinate)?

I believe it is in principle possible to get merged into SecondaryVerticesHelix.cc. But the Helix.cc is pretty sizable, and I believe many functions can be used in other down stream analysis. The code is largely copied from STAR with adjustments to be adaptable to edm4eic constainers. I think it is probably preferrable to keep them separated so it is easier to maintain.

I wasn't sure that I understand completely about your comment "I also see you provide Config structure, but don't expose the parameters in the factory." Do you suggest to explicitly put them out in reco.cc when creating this factory? I see there, some factories do this way, some don't. I am fine with either requirement.

Regarding the b_field, Yes, I was thinking about the way you suggested too. Let me try to update this part soon. Thanks

I believe it is in principle possible to get merged into SecondaryVerticesHelix.cc. But the Helix.cc is pretty sizable, and I believe many functions can be used in other down stream analysis. The code is largely copied from STAR with adjustments to be adaptable to edm4eic constainers. I think it is probably preferrable to keep them separated so it is easier to maintain.

Maybe it's for the best to keep it separate, especially if we find that it has questions about authorship/license. But in principle, if left as is, we may have to relocate it at some point.

I wasn't sure that I understand completely about your comment "I also see you provide Config structure, but don't expose the parameters in the factory." Do you suggest to explicitly put them out in reco.cc when creating this factory? I see there, some factories do this way, some don't. I am fine with either requirement.

I did not mean this. See my inline comment above, I hope that will clarify.

Hi Dmitri,

The PR has been updated with new commits addressing the magnetic field input and the parameters declaration in Config file. Thanks

/xin

Also, if I want to test this, which sample would you recommend? epic_craterlake/SIDIS/D0_ABCONV/pythia8.306-1.0/18x275/hiDiv? Is there a benchmark script that we can put in?

I think, this is in good shape. Some last comments. Could you clarify on Wouter's question regarding the Helix code authorship.

Yes, I am contacting STAR to clarify how to best address the authorship issue. Will follow up soon.

Also, if I want to test this, which sample would you recommend? epic_craterlake/SIDIS/D0_ABCONV/pythia8.306-1.0/18x275/hiDiv? Is there a benchmark script that we can put in?

I think this should be a good sample (or any DIS event sample). I am not familiar with the benchmark script. Maybe we can follow up along later to set it up.

Since there was no benchmark provided, I vibe coded a simple analysis for myself. It looks like this code enables us to resolve kaon decays: